Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rrd improvements #326

Merged
merged 1 commit into from
Aug 16, 2023
Merged

Conversation

mambelli
Copy link
Contributor

@mambelli mambelli commented Aug 7, 2023

Code improvements in RRD handling code, no specific bug fix:

  • Improved path handling and directory creations
  • creating also parent directories,
  • eliminated nested os.path.join calls,
  • consolidated tmp2final verifyHelper functions,
  • eliminated global variable use,
  • use os.path.join instead of string concatenation with /

Also improved docstrings

The exception improvements changes have been separated out in PR #330

@mambelli mambelli marked this pull request as ready for review August 13, 2023 09:29
@namrathaurs namrathaurs self-requested a review August 14, 2023 21:47
Copy link
Contributor

@namrathaurs namrathaurs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost all the suggestions are similar to each other; with only a change in the respective variables depending on what's being used. Please consider these comments if valid and applicable. If not valid, I'd be interested to know why they are not.

lib/rrdSupport.py Outdated Show resolved Hide resolved
lib/rrdSupport.py Outdated Show resolved Hide resolved
frontend/glideinFrontendMonitoring.py Outdated Show resolved Hide resolved
frontend/glideinFrontendMonitoring.py Outdated Show resolved Hide resolved
factory/glideFactoryMonitorAggregator.py Outdated Show resolved Hide resolved
factory/glideFactoryMonitorAggregator.py Outdated Show resolved Hide resolved
factory/glideFactoryMonitorAggregator.py Outdated Show resolved Hide resolved
frontend/glideinFrontendMonitorAggregator.py Outdated Show resolved Hide resolved
frontend/glideinFrontendMonitorAggregator.py Outdated Show resolved Hide resolved
frontend/glideinFrontendMonitorAggregator.py Outdated Show resolved Hide resolved
@mambelli mambelli changed the title rrd improvements and better exception handling rrd improvements Aug 15, 2023
Fixed bug: rrdtools_exe.dump() was wrongly decoding the output of subprocessSupport.iexe_cmd() which is already a string

Code improvements, no specific bug fix:
creating also parent directories,
eliminated nested os.path.join calls,
better docstrings,
consolidated tmp2final verifyHelper functions,
eliminated global variable use,
use os.path.join instead of string concatemation with /
Copy link
Contributor

@namrathaurs namrathaurs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good and can be merged!

@mambelli mambelli merged commit f6dab66 into glideinWMS:master Aug 16, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants